feat: add --dry-run to release create and release delete - #699

Dry run fails for version-controlled (CaC) projects when --git-ref is omitted.

gitReferenceKey stays "" here when neither --git-ref nor --git-commit is given. DeploymentProcesses.Get tolerates that (the SDK falls back to gitPersistenceSettings.DefaultBranch()), but determineReleaseVersion → resolveVersioningStrategy → Deployments.GetDeploymentSettings(project, "") does not: for a CaC project the DeploymentSettings link is git-templated, and the SDK expands it with no gitRef parameter (its doc says "If the project is version controlled you'll need to specify a gitRef"), producing a malformed path/404. So release create -p CacProject -c SomeChannel --dry-run errors, while the real (non-dry-run) create succeeds because the server defaults to the default branch. It also means the process template and the deployment settings could come from different refs.

Suggested fix: mirror the SDK — when the project is version-controlled and gitReferenceKey is empty, default it to project.PersistenceSettings.(projects.GitPersistenceSettings).DefaultBranch() before using it.

Actioned in b30d1fe, with a regression test added in 1b4fc87.

buildReleasePreview now pins gitReferenceKey to project.PersistenceSettings.(projects.GitPersistenceSettings).DefaultBranch() when the project is version-controlled and neither --git-ref nor --git-commit was given, exactly as you suggested. Pinning it at that one point (rather than only before the settings call) also keeps the process template and the deployment settings on the same ref, which was the second half of the problem.

I confirmed the diagnosis rather than taking it on trust: with the pin removed, the new test fails with

expected: "/api/Spaces-1/projects/Projects-87/main/deploymentsettings"
actual  : "/api/Spaces-1/projects/Projects-87//deploymentsettings"

so the malformed path is real, and DeploymentProcesses.Get did resolve fine without it — the asymmetry you described.

Test: TestReleaseCreate_DryRun/"dry run for a config-as-code project without --git-ref reads everything from the default branch". It drives the whole command against the mock server and asserts the exact request sequence, so the main segment in both the template and the settings URLs is load-bearing.

Not covered: a version-controlled project whose PersistenceSettings doesn't type-assert to GitPersistenceSettings. That still falls through with an empty ref rather than erroring; I left it alone because it shouldn't be reachable.

Pre-existing nil-pointer panic made more visible by this PR (line 811, just below this hunk).

This condition enters the donor branch when DonorPackageStepID != nil even if DonorPackage == nil, and line 811 then dereferences versioningStrategy.DonorPackage.PackageReference unconditionally — a nil-pointer panic in interactive mode.

Your new determineReleaseVersion explicitly acknowledges that state is real ("a donor step with no package reference; nothing to read a version from") and handles it safely. Since AskQuestions is being refactored here anyway, worth guarding the interactive path the same way: if versioningStrategy.DonorPackage != nil { donor branch } else if versioningStrategy.DonorPackageStepID != nil { skip/ask }.

Actioned in cef406d, test in e03b697.

The condition now branches on DonorPackage first, and a bare DonorPackageStepID falls into an explicit no-op branch with a comment, so the version is left blank for the server to assign — the same conclusion determineReleaseVersion reaches on the automation path. I kept the branch rather than folding it into the Template case so the three states stay visible in the code.

Verified the panic was real before fixing it: with the old DonorPackageStepID != nil || DonorPackage != nil condition restored, the new test dies with

panic: runtime error: invalid memory address or nil pointer dereference
  create.AskQuestions ... create.go:847

Test: TestReleaseCreate_AskQuestions_RegularProject/"a donor step with no donor package leaves the version to the server rather than panicking". It asserts no version question is asked and options.Version stays empty.

One judgement call worth flagging: leaving the version blank means the server assigns it, which is the best available behaviour here but is silent. If you'd rather the interactive path told the user ("this project versions from a step with no package reference; the server will assign the version"), say so and I'll add the line — I didn't want to invent user-facing copy in a fix commit.

Interactive dry run echoes an automation command that mutates (line 313).

In interactive mode the dry-run check happens after the Automation Command: line is printed, and that echoed command deliberately omits --dry-run. You flagged this as an open question — my vote: it's a footgun. The natural reading of "I ran a rehearsal, here's the equivalent automation command" is that the echoed command reproduces the rehearsal; pasting it into CI creates a real release. Either append --dry-run when the flag is set, or annotate the line (e.g. Automation Command (add --dry-run to rehearse): ...) so the asymmetry is explicit.

Actioned in 2b97415, test in e03b697.

Agreed it's a footgun — an echoed command that quietly upgrades a rehearsal into a real create is worse than no echo at all. Took the first of your two options: resolvedFlags.DryRun.Value is now carried from the run's own flag and DryRun is passed to GenerateAutomationCmd, so the echoed line ends ... --dry-run --no-prompt and reproduces what just ran. The annotation option would have left the copy-paste hazard in place and relied on the reader noticing the parenthetical.

Test: TestReleaseCreate_DryRun_Interactive in create_test.go — a new interactive test through the root command (there wasn't one for release create; the existing interactive tests call AskQuestions directly and never reach the echo). It supplies everything on the command line so the Q&A asks nothing, then asserts the exact stdout, including the automation command line and the preview that follows it.

One cosmetic residual I noticed while asserting that output: because the echo ends with a single newline and dryrun.Header doesn't lead with one, the automation command and DRY RUN: no changes will be made in Octopus. end up on adjacent lines with no blank between them. Readable, but tighter than the rest of the dry-run output. Happy to add a separating newline if you want it; I left the formatting alone rather than changing shared output.

--ignore-channel-rules --dry-run shows a plan filtered by the rules the real create would ignore.

BuildPackageVersionBaselineForChannel always applies the channel's version rules, and options.IgnoreChannelRules is never consulted on this path. With --ignore-channel-rules, the server resolves/validates without the rules, so the previewed package versions (and a donor-derived Version) can differ from the release actually created — e.g. a rule pinning 1.x makes the preview show 1.9 (or unknown if nothing matches) while the real create picks 2.0. The interactive Q&A has the same behavior, so this may be acceptable — but a dry run is specifically a promise about what would happen, so consider skipping the rule filter (or noting it in the output) when IgnoreChannelRules is set.

Partly actioned in 1b90bed, and I want a decision from you on the rest.

The finding holds: BuildPackageVersionBaselineForChannel injects the channel's VersionRange/Tag/VersionTagRegex into every feed query and options.IgnoreChannelRules is never consulted on this path, so the previewed versions (and a donor-derived Version) are rule-filtered while the create the user is rehearsing is told to ignore those rules.

What I did: took the "note it in the output" half. The table now prints a dimmed caveat under the package list when IgnoreChannelRules is set, saying the versions were resolved with the channel's rules applied and the server may pick differently. JSON already carries IgnoreChannelRules: true, so a machine consumer can see the same thing without new copy.

What I deliberately did not do: skip the rule filter. The mechanical change is one line —

baseline, err := packages.BuildPackageVersionBaseline(octopus, deploymentProcessTemplate.Packages, nil)

(the callback is nil-checked, so passing nil gives "latest in feed, no rules"). I didn't take it because it only improves the preview if the server, under ignoreChannelRules, also selects versions without the rules rather than merely skipping rule validation. If it's validation-only, that change would make the preview wrong in the common case, having previously been right.

This is the same question as the --ignore-channel-rules blind spot you raised on #695's package diagnosis, and I'd rather both paths make the same assumption. The difference in stakes: there, a wrong assumption mislabels an error message; here it changes the plan the user is trusting.

Open question: does ignoreChannelRules on the create-release API skip rule-constrained package selection, or only rule validation? I can't answer it from this repo and I haven't tested it against a server. If it skips selection too, I'll make the one-line change here and drop the caveat note; if it's validation-only, the current note is the right answer and I'll leave it.

release delete --dry-run -f json emits ANSI-colored human text, not JSON.

--output-format is a persistent root flag, so it's accepted here, and printDeletePlan ignores it — a CI consumer that parses -f json output gets DRY RUN: ... prose (with color codes) on stdout. The PR description's safety argument ("a CI step parsing JSON cannot confuse a plan for a result, because the plan carries DryRun: true") only holds for release create. The non-dry-run delete has no JSON output either, so this is consistent — but the dry-run feature is aimed squarely at CI, so a small DeletePlan{DryRun: true, Project, Versions} JSON branch mirroring printReleasePreview would close the gap.

Actioned in 98b908b.

Added the DeletePlan{DryRun, Project, Versions} branch you described. printDeletePlan now takes the output format, reads it from the persistent root flag, and marshals JSON when -f json was asked for, keeping DryRun as the first field so it mirrors ReleasePreview and a CI consumer cannot read a plan as a result. The human branch is unchanged apart from the empty case (your other comment).

Test: TestReleaseDelete/"noprompt: dry run with json output emits a machine readable plan flagged as a dry run" asserts the exact payload {"DryRun":true,"Project":"Fire Project","Versions":["2.0"]} — exact-match, so colour codes or prose leaking back in would fail it.

Agreed on your framing that the non-dry-run delete still has no JSON output; I left that alone as out of scope for this PR. It does mean release delete -f json without --dry-run still prints prose, so the two differ in shape — worth an issue if we want -f json to be uniform across the command.

Dry run prints nothing at all when no versions match.

When none of the requested versions exist (octopus release delete -p X -v 9.9 --no-prompt --dry-run), the len(releasesToDelete) == 0 return just above exits 0 with zero output — the caller can't tell "the dry run ran and would delete nothing" from "the flag was ignored". The live command shares the silent no-op, but for a rehearsal an explicit line matters more; consider printing the dry-run header plus "Would delete 0 release(s) — no releases matched" before that early return when flags.DryRun.Value is set.

Actioned in 98b908b.

The dry-run branch now sits before the len(releasesToDelete) == 0 early return, so a rehearsal that matched nothing prints

DRY RUN: no changes will be made in Octopus.

Would delete 0 release(s) from project Fire Project: no releases matched.

DRY RUN: no releases were deleted.

and in JSON {"DryRun":true,"Project":"...","Versions":[]}. The interactive confirmation guard became len(releasesToDelete) > 0 && !flags.DryRun.Value, which keeps the non-dry-run behaviour identical: zero matches still skips the prompt and exits 0 silently, because the old early return and the surviving len == 0 check land in the same place.

Test: TestReleaseDelete/"noprompt: dry run says so when nothing matches" asserts that exact output.

Left the live command's silent no-op alone, per your read that it matters less there.

Custom Field rows print in nondeterministic order.

Iterating preview.CustomFields (a map) shuffles the rows run-to-run for a release with two or more custom fields — noisy for humans diffing two dry runs, and untestable with an exact-match assertion like the ones in create_test.go. Sort the keys before appending rows. (Same pattern exists at line 291 for the automation command, but this one is new.)

Actioned in df350e6, test in 04f2b12.

Added a small sortedKeys helper and used it for both the preview rows and the automation command line you pointed at (line 291) — same bug, same fix, and the automation command is worth stabilising for the same diff-two-runs reason.

Test: TestReleaseCreate_DryRun/"dry run prints custom fields in a stable order" supplies three fields in non-sorted order and asserts the exact table, which is only possible now that the order is deterministic — your point about untestability was the reason there was no such assertion before.

SetDryRun(false) is a silent no-op — the signature promises a toggle the implementation doesn't deliver.

Once armed, c.dryRun short-circuits every later call, so a future caller writing SetDryRun(false) (say, a test harness or a multi-command flow re-using the factory) leaves the guard installed and every mutation blocked, with nothing to say why. Since the guard is deliberately one-way for the process lifetime, make the API say so: either rename to something like EnableDryRunGuard() with no parameter, or document on the interface that disabling is not supported.

Actioned in 4028685.

Took the rename rather than the doc-only option: SetDryRun(enabled bool) is now EnableDryRunGuard() with no parameter, on both the ClientFactory interface and *Client, plus the stub factory and the one caller in root.go. The interface comment now says outright that the guard is one-way for the lifetime of the process. With no parameter there is no false to silently swallow, so the failure mode you described stops being expressible.

Test: TestClientFactory_EnableDryRunGuard_RefusesMutatingRequests (renamed with it) still covers the arming path.

recordingRoundTripper is defined twice in this PR — identically here and in pkg/apiclient/client_factory_test.go. Worth hoisting into test/testutil (next to MockHttpServer) so the next dry-run-covered command's tests don't make a third copy.

Actioned in fbd2077.

Hoisted it to testutil.RecordingRoundTripper in test/testutil/testutil.go, next to MockHttpServer as you suggested, and deleted both local copies (pkg/dryrun/dryrun_test.go and pkg/apiclient/client_factory_test.go now use the shared one). Gave it a doc comment saying what it's for, since the point is the next dry-run-covered command finds it instead of writing a third.

Both existing tests still pass unchanged otherwise: TestGuardRoundTripper and TestClientFactory_EnableDryRunGuard_RefusesMutatingRequests.

Preserve --package-version in the preview when no channel is supplied.

DefaultPackageVersion is not copied into ReleasePreview, and the no-channel branch returns before applying it to a baseline. As a result, release create -p Example --package-version 9.9.42 --no-prompt --dry-run produces exactly the same plan as omitting the flag, in both table and JSON output, even though the real request sends PackageVersion: "9.9.42". I reproduced this with a focused test of buildReleasePreview and both output formats.

Please carry the default package version as a separate preview field and display it even when the channel/package selection is deferred to the server. Add no-channel tests with --package-version, including a case combined with a per-package override.

Actioned in 2044a2a.

Confirmed the reproduction: DefaultPackageVersion was never copied into ReleasePreview, and the no-channel branch returns before BuildPackageVersionOverrides would have applied it, so the flag left no trace in either output format while the real request still sends PackageVersion.

ReleasePreview now carries DefaultPackageVersion as its own field (placed after Version, json:",omitempty"), populated straight from options.DefaultPackageVersion before either early return. The table prints a Default Package Version row whenever it's set — including when a channel was supplied and the resolved Packages table already reflects it. Slightly redundant in that case, but it states what was asked for rather than only what it resolved to, and the alternative (show it only when deferred) makes the output shape depend on something the reader can't see.

Tests, all in TestReleaseCreate_DryRun:

• "dry run without a channel still reports the default package version" — table output, --package-version alone.
• "dry run without a channel reports the default package version alongside per-package overrides" — table, combined with --package pterm:1.2, which also shows the existing Package overrides: block.
• "dry run json output without a channel carries the default package version and overrides" — exact JSON, {"DryRun":true,...,"DefaultPackageVersion":"9.9.42","PackageOverrides":["pterm:1.2"],...}.